Skip to content

Conversation

@lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Sep 12, 2025

Main purpose was to change the library to a more maintained version from @Nadahar.
This made it also possible to change from data polling to events, making the binding a bit more snappy with updates.
Improved scheduled task disposal.

Signed-off-by: Leo Siepel <leosiepel@gmail.com>
@lsiepel lsiepel requested a review from kaikreuzer as a code owner September 12, 2025 08:30
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Sep 12, 2025

<config-description uri="thing-type:chromecast:device">
<parameter name="ipAddress" type="text" required="true">
<parameter name="host" type="text" required="true">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the reviewer, the hostname can be used, so i think ipAddress is the wrong name, but it is also breaking, so i have some doubts to change it or not. I can also update the docs, that ipAddress can also contain a hostname, but meh.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't it at least be explained in the parameter description? That wouldn't break anything.

@lsiepel lsiepel requested a review from a team September 12, 2025 09:50
Comment on lines 62 to 69
List<MediaStatus> statuses = event.getData(MediaStatusResponse.class).getStatuses();
if (statuses != null && statuses.size() > 1) {
logger.warn("Received multiple media statuses, this is not supported. Statuses: {}", statuses);
} else if (statuses != null && !statuses.isEmpty()) {
statusUpdater.updateMediaStatus(statuses.getFirst());
} else {
statusUpdater.updateMediaStatus(null);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
List<MediaStatus> statuses = event.getData(MediaStatusResponse.class).getStatuses();
if (statuses != null && statuses.size() > 1) {
logger.warn("Received multiple media statuses, this is not supported. Statuses: {}", statuses);
} else if (statuses != null && !statuses.isEmpty()) {
statusUpdater.updateMediaStatus(statuses.getFirst());
} else {
statusUpdater.updateMediaStatus(null);
}
MediaStatusResponse mediaStatusResponse = event.getData(MediaStatusResponse.class);
List<MediaStatus> mediaStatuses = mediaStatusResponse == null ? null : mediaStatusResponse.getStatuses();
if (mediaStatuses == null) {
statusUpdater.updateMediaStatus(null);
} else {
for (MediaStatus mediaStatus : mediaStatuses) {
statusUpdater.updateMediaStatus(mediaStatus);
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will be multiple statuses at times. There's no point in "rejecting" that, just apply them in order..?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I remember, certain "actions" involve transitions that produce more than one media status in sequence.

case APPEVENT:
logger.debug("Received an 'APPEVENT' event, ignoring");
case RECEIVER_STATUS:
statusUpdater.processStatusUpdate(event.getData(ReceiverStatusResponse.class).getStatus());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
statusUpdater.processStatusUpdate(event.getData(ReceiverStatusResponse.class).getStatus());
ReceiverStatusResponse receiverStatusResponse = event.getData(ReceiverStatusResponse.class);
statusUpdater.processStatusUpdate(receiverStatusResponse == null ? null : receiverStatusResponse.getStatus());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite what Eclipse's nullness logic claims, getData() very much can return null.

@lsiepel lsiepel marked this pull request as draft September 13, 2025 19:33
Ravi Nadahar added 2 commits September 15, 2025 19:59
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
@Nadahar
Copy link
Contributor

Nadahar commented Oct 22, 2025

@lsiepel I guess this one drowned a bit among everything else. I'll try to look at it again.

I got "sidetracked" a bit by starting to look at the Core mDNS implementation. It's far from ideal, but I don't quite know how to fix it. In a way, the way jmdns works is problematic and somewhat unclear.

jmdns doesn't cache everything it sees, it only starts caching when it has a listener registered for a specific service. It's unclear to me if registering as a listener is enough for jmdns to actively probe (broadcast requests for devices providing that service to report back), or if you actually have to call getServiceInfo() or requestServiceInfo() for active probing to take place. The list() method is also confusing, because the documentation only says: "Returns a list of service infos of the specified type".

The implementation for list() has a code comment that says:

        // Implementation note: The first time a list for a given type is
        // requested, a ServiceCollector is created which collects service
        // infos. This greatly speeds up the performance of subsequent calls
        // to this method. The caveats are, that 1) the first call to this
        // method for a given type is slow, and 2) we spawn a ServiceCollector
        // instance for each service type which increases network traffic a
        // little.

A ServiceCollector is what makes jmdns cache information about a particular service, one is also created when you register a listener. But, there is no info about how long this ServiceCollector will live with a call to list(), since the ones created when registering a listener lives until you unregister the listener.... but there is no such "link" by a simple call to list(). So, I assume it lives for a certain period, but it's just guesswork.

And, I'm still not sure exactly when an "active probe" for the service is being done. Ideally, you'd want jmdns to do an "active probe" for the Chromecast service when the binding starts, and then keep a ServiceCollector as long as the binding is running, which would mean that information could be retrieved from the cache.

Worse, OH's MDNSClient doesn't expose getServiceInfo() and requestServiceInfo(), which are the two methods that I'm pretty confident will perform an "active probe".

On top of that, OH spawns one jmdns client for each network interface, and then forwards "requests" to all of these.

It's just complicated to know how to handle this efficiently and correctly, but it's clear to me that it's needed and that mDNS use in OH in general causes issues for users. I'm just not sure what the solution is.

@lsiepel
Copy link
Contributor Author

lsiepel commented Oct 22, 2025

There are so many topics to work on, so this one was also not high on my list. I looked at your PR, merged it and now i have been fixing several other PR's and reviews i had lingering around.

Your comment rather looks like a generic openHAB mDNS comment and not specific to chromecast. You say that mDNS usage leads to problems for many users. Do you have specific cases in mind? Maybe if we look after a specific case we can narrow it down, or make the problem smaller/reduce it.

@Nadahar
Copy link
Contributor

Nadahar commented Oct 22, 2025

Your comment rather looks like a generic openHAB mDNS comment and not specific to chromecast.

The comment is generic, but it's also tied to figuring out how to correctly handle mDNS here. As I've said before, I think one of the benefits of switching library is that it will no longer spin up its own jmdns instance that makes a mess over everything, because it's working "in parallel" with OH's jmdns instance, which means that "the OH computer" will make multiple, diverging responses to requests from other devices. With the current library, you don't have to think about mDNS, since it doesn't give you any choice, it will so what it wants to do. But this also causes problems. "Getting control" of mDNS also means that we now have to sort of how to handle it correctly.

You say that mDNS usage leads to problems for many users. Do you have specific cases in mind? Maybe if we look after a specific case we can narrow it down, or make the problem smaller/reduce it.

There have been multiple forum threads about problems, where jmdns is high on the list of suspects. I've dug into thread dumps where there are loads of jmdns threads either deadlocked or "slow walking", waiting for other things, delaying "everything" and making OH slow or stop responding. It's hard to pinpoint exactly what leads up to these situations, when a thread dump is just a snapshot of what is going on at any given moment, but I suspect that things aren't handled efficiently and optimally, that things that should only be done once are done over and over again for example.

mDNS is also "slow by nature" in that it's based on a model of broadcast -> reply, where the broadcast should be repeated 3 times (I think) with a given interval, and participants can respond basically "at any time", so you have to wait several seconds to be sure that you've captured all replies. Therefore, it's even more important that things aren't done more often than they should, with waiting times and all what follows.

There have also been people that have reported "mDNS storms" on their networks when OH runs, and there are others that just want to disable it because it causes instability and slowness (hence the focus on disabling add-on finders). I'm not saying that OH is to blame for all this, mDNS is a "problematic standard" in itself, where it doesn't seem to me like the designers really though things through properly. So, there are all kinds of "ad hoc" solutions at work here, both in OSes and in devices.

I've tried to find solutions to all this, but have mostly been banging my head against the wall. The mDNS standard is hard to read, because it keeps referencing the DNS standard, so you really need to know them both to get an overview. The jmdns code is written in a way that is very hard to "follow" for me, so I often just have to give up trying to get to the bottom - both because I don't fully know the standards (aka what it should be doing) and I can't figure out what it is doing.

@Nadahar
Copy link
Contributor

Nadahar commented Oct 23, 2025

A lot is going on with mDNS, but we're normally hiding it from the log. Try setting javax.jmdns to DEBUG, and try to make sense of what you see 🙄

I'm seeing a lot of double events from the Chromecast in the log, so either there's a "double event subscription" going on, or there's some problem with the library.

edit: The audio sink is also registered twice according to the UI, so I'm assuming that the binding is a bit too eager.

edit2: I find two of everything, channels, cast devices, input processing threads. So, I assume that the binding somehow creates two CastDevices for one actual device. I haven't found it yet though.

@Nadahar
Copy link
Contributor

Nadahar commented Oct 23, 2025

I'm seeing a lot of double events from the Chromecast in the log, so either there's a "double event subscription" going on, or there's some problem with the library.

🤦 Or could it be that I had two Things configured for the same device? 😊

@Nadahar
Copy link
Contributor

Nadahar commented Oct 24, 2025

I'm trying to implement some additional events, and I'm looking at the multizone stuff. I've never taken the trouble to figure out what it is/how it's used. Do you have any idea? Is that something that would be useful for OH?

MultizoneStatus carries two pieces of information:

  • A list of Devices that are part of the zone
  • A boolean that states whether the zone is "multi channel" or not.

Again, I have no idea what this is, but I'm imagining that it might be relevant for OH it people use Chromecasts as audio sinks.. where maybe you can cast something to an entire group?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement or new feature for an existing add-on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants